Add support for php7 and get all tests running #53

andrerom · 2016-04-14T11:43:51Z

~~Might fail on a few things.~~ passing

Note: PHP7 might never be fully supported with legacy, this just makes sure it can be used for dev use, and is a first step towards getting legacy running on php7.

emodric · 2016-04-15T08:31:09Z

bundle/DependencyInjection/EzPublishLegacyExtension.php

@@ -59,7 +59,7 @@ public function load(array $configs, ContainerBuilder $container)
        $processor = new ConfigurationProcessor($container, 'ezpublish_legacy');
        $processor->mapConfig(
            $config,
-            function (array &$scopeSettings, $currentScope, ContextualizerInterface $contextualizer) {
+            function (array $scopeSettings, $currentScope, ContextualizerInterface $contextualizer) {


Is this something that could break on PHP 5.x?

Don't see how, the variable is not modified inside the closure.

Yep, just wanted to confirm.

No, it won't break.

Might fail on a few things.

Not needed in this case as it is not modified, and value given.

andrerom · 2016-08-15T11:31:18Z

review ping @emodric @lolautruche

lolautruche · 2016-08-15T11:34:41Z

bundle/LegacyMapper/SiteAccess.php

@@ -47,6 +47,10 @@ public function onBuildKernelWebHandler(PreBuildKernelWebHandlerEvent $event)
        $request = $event->getRequest();
        $uriPart = array();

+        // @todo Remove when, if ever, legacy does not use deprecated constructor names
+        // triggers initial autoload of eZSiteAccess and sileces deprecation warning
+        @eZSiteAccess::TYPE_DEFAULT;


I don't get this...
As for the comment, you just need to silent E_DEPRECATED errors in php.ini

We can, but for symfony 3.0 support we actually want to have deprecation errors make test fail on other things. Hence the selective silencing just when loading legacy classes, as we kind of want to know about anything else.

but for symfony 3.0 support we actually want to have deprecation errors make test fail on other things

E_DEPRECATED !== E_USER_DEPRECATED

😉

true, ok I'll see if I get that running then.

fixed, but by just switching error level for this test only, want to get other deprecation errors if there are any. constructor is perhaps the only we shouldn't do anything about for BC.

emodric · 2016-08-18T08:48:17Z

composer.json

+        "matthiasnoback/symfony-dependency-injection-test": "^1.0",
+        "phpunit/phpunit": "~4.7",
+        "mikey179/vfsStream": "~1.1.0",
+        "mockery/mockery": "^0.9@dev"


Why @dev ?

mockery/mockery@0.9.5...0.9

Okay, but isn't ^0.9 enough?

it can be, but then we won't pull in those fixes as it will use the tag.

It will pull 0.9.5, that's not enough?

does not sound like you checked the link, but removed it

does not sound like you checked the link, but removed it

I did, but the link shows diff between 0.9 and 0.9.5 versions. ^0.9 will pull in 0.9.5, so I'm not sure what the issue is :)

emodric · 2016-08-18T08:49:46Z

Aside from inline comment, +1.

emodric · 2016-08-18T09:25:55Z

bundle/Cache/SwitchableHttpCachePurger.php

@@ -42,12 +42,12 @@ public function purgeAll()
        $this->gatewayCachePurger->purgeAll();
    }

-    public function purgeForContent($contentId)
+    public function purgeForContent($contentId, $locationIds = array())


Isn't this a breaking change? If so, we should require eZ Kernel >=6.3.2|>=6.4.2 (when 6.4.2 is released) in composer.json, right?

yes, but adding optional argument won't break the contract. In retrospective it was the kernel that broke the contract here.

So besides the fact that kernel should not have done this change, I think this fix is ok as is, it will work on older kernels also.

So besides the fact that kernel should not have done this change, I think this fix is ok as is, it will work on older kernels also.

Agreed.

andrerom · 2016-08-18T10:58:46Z

looks ok to you as well now @lolautruche ?

EDIT: Hmm, failing now, might be because of the d0a0c33 change edi asked me to do.

lolautruche · 2016-08-18T11:19:36Z

+1

Weird that it doesn't fail with PHP 5.4

…tor names as it will most likely never be changed

…r to dump config here

andrerom · 2016-08-18T12:27:01Z

Found it, we allow SensioDistributionBundle 5.0 now for work towards Sf 3.0 support, it supports PHP 5.5+, and it does not contain Configurator as needed by LegacyBridge.

Added it to composer.json as it is a clear requirement.

Thanks for reviews, merging :)

andrerom changed the title ~~Update travis to test on php7 also~~ Update travis to test on php7 Apr 14, 2016

andrerom changed the title ~~Update travis to test on php7~~ Run tests on php7 Apr 14, 2016

emodric reviewed Apr 15, 2016
View reviewed changes

andrerom added 2 commits August 15, 2016 12:00

Update travis to test on php7 also

331ba65

Might fail on a few things.

[PHP7] Change EzPublishLegacyExtension to not expect referance

e3dce52

Not needed in this case as it is not modified, and value given.

andrerom force-pushed the test-on-php7 branch from c8fd95f to b71fc1c Compare August 15, 2016 10:11

[Tests] Send closure as expected to avoid TypeError on php7

5acfac2

andrerom force-pushed the test-on-php7 branch 2 times, most recently from 0d95f40 to 087efc7 Compare August 15, 2016 11:22

andrerom changed the title ~~Run tests on php7~~ Add support for php7 and get all tests running Aug 15, 2016

lolautruche reviewed Aug 15, 2016
View reviewed changes

andrerom force-pushed the test-on-php7 branch 2 times, most recently from 90d6969 to e7b7cd1 Compare August 15, 2016 13:00

emodric reviewed Aug 18, 2016
View reviewed changes

andrerom added 5 commits August 18, 2016 13:25

[Composer] Bump dependencies to versions suitable for php7

ab5ab09

[HttpCache] Adapt SwitchableHttpCachePurger for EZP-25003

3a855c8

[Tests] Silence deprecation errors from legacy in regards to construc…

da8e950

…tor names as it will most likely never be changed

[Travis] Enable caching of Composer packages

2dab648

[Composer] Require SensioDistributionBundle 3|4 as we use Configurato…

b6b85e8

…r to dump config here

andrerom force-pushed the test-on-php7 branch from 2e8698d to b6b85e8 Compare August 18, 2016 11:52

andrerom merged commit 2a36616 into master Aug 18, 2016

andrerom deleted the test-on-php7 branch August 18, 2016 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for php7 and get all tests running #53

Add support for php7 and get all tests running #53

andrerom commented Apr 14, 2016 •

edited

Loading

emodric Apr 15, 2016

andrerom Apr 15, 2016

emodric Apr 15, 2016

lolautruche Aug 15, 2016

andrerom commented Aug 15, 2016

lolautruche Aug 15, 2016

andrerom Aug 15, 2016 •

edited

Loading

lolautruche Aug 15, 2016 •

edited

Loading

andrerom Aug 15, 2016

andrerom Aug 15, 2016 •

edited

Loading

emodric Aug 18, 2016

andrerom Aug 18, 2016

emodric Aug 18, 2016

andrerom Aug 18, 2016

emodric Aug 18, 2016

andrerom Aug 18, 2016 •

edited

Loading

emodric Aug 18, 2016

emodric commented Aug 18, 2016

emodric Aug 18, 2016

andrerom Aug 18, 2016 •

edited

Loading

emodric Aug 18, 2016

andrerom commented Aug 18, 2016 •

edited

Loading

lolautruche commented Aug 18, 2016

andrerom commented Aug 18, 2016

Add support for php7 and get all tests running #53

Add support for php7 and get all tests running #53

Conversation

andrerom commented Apr 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrerom commented Aug 15, 2016

Choose a reason for hiding this comment

andrerom Aug 15, 2016 • edited Loading

Choose a reason for hiding this comment

lolautruche Aug 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrerom Aug 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrerom Aug 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emodric commented Aug 18, 2016

Choose a reason for hiding this comment

andrerom Aug 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrerom commented Aug 18, 2016 • edited Loading

lolautruche commented Aug 18, 2016

andrerom commented Aug 18, 2016

andrerom commented Apr 14, 2016 •

edited

Loading

andrerom Aug 15, 2016 •

edited

Loading

lolautruche Aug 15, 2016 •

edited

Loading

andrerom Aug 15, 2016 •

edited

Loading

andrerom Aug 18, 2016 •

edited

Loading

andrerom Aug 18, 2016 •

edited

Loading

andrerom commented Aug 18, 2016 •

edited

Loading